History: Expose and call SetContextMetadata in readonly operations#9240
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
nikki-dag
force-pushed
the
nikki/readonly-context-metadata
branch
from
7497389 to
4221700
Compare
| } | ||
| } | ||
|
|
||
| if len(request.Commands) == 0 && len(request.Messages) > 0 { |
There was a problem hiding this comment.
hmm... do we have any action metric to emit if both are empty? Is workflow task heartbeat an action?
There was a problem hiding this comment.
Thanks for catching this!
yycptt
left a comment
yycptt
left a comment
There was a problem hiding this comment.
We need some tests ensuring those two apis handler have the context populated.
nikki-dag
force-pushed
the
nikki/readonly-context-metadata
branch
3 times, most recently
from
f2a2de6 to
c3b8c49
Compare
nikki-dag
force-pushed
the
nikki/readonly-context-metadata
branch
from
c3b8c49 to
a522a1e
Compare
There was a problem hiding this comment.
Is it possible to change the existing tests in history_engine_tests.go?
| // Context metadata is automatically set during mutable state transaction close. For RespondWorkflowTaskCompleted | ||
| // with no commands (e.g., workflow task heartbeat or only readonly messages like `update.Rejection`), the transaction | ||
| // is never closed. We explicitly call SetContextMetadata here to ensure workflow metadata is populated in the context. | ||
| workflowLease.GetMutableState().SetContextMetadata(ctx) |
There was a problem hiding this comment.
fwiw, I don't think the mutable state you get from workflowLease.GetMutableState() will always be the latest mutable state because it's a separate copy in this workflowLease object, while the real one is in workflowLease.Context. Doesn't really matter for your case the can be confusing?
Why not use the ms variable instead and do it before the defer() method? similar to what you have in the Query api
2 participants
What changed?
SetContextMetadata()public in MutableState interfaceMetadataKeyWorkflowType,MetadataKeyWorkflowTaskQueue)ContextHasMetadata()helper function to check if context has metadata support-
QueryWorkflow: Now explicitly callsSetContextMetadata()since queries never close transactions (readonly operation)-
RespondWorkflowTaskCompleted: CallsSetContextMetadata()when there are only messages (e.g., update rejections) and no commands, since these also don't close transactionsContextHasMetadata()helper andSetContextMetadata()functionalityWhy?
Context metadata (workflow type, task queue) is automatically populated when mutable state transactions close. However, readonly operations never close transactions, so the metadata was missing from their contexts.
This change ensures that successful readonly operations (queries, update rejections) also have workflow metadata populated in their contexts
How did you test it?
Note
Medium Risk
Touches core history API request paths and mutable state interfaces; while behavior is additive (context tagging), incorrect invocation or key usage could affect observability/metrics and tests across multiple workflows.
Overview
Ensures workflow context metadata (workflow type/task queue) is consistently populated even for readonly paths that don’t close mutable-state transactions.
This exposes
MutableState.SetContextMetadata(ctx)(renaming the internal helper), standardizes metadata keys viacontextutil.MetadataKeyWorkflowType/MetadataKeyWorkflowTaskQueue, addsContextHasMetadata()for debugging, and explicitly callsSetContextMetadatainQueryWorkflowand inRespondWorkflowTaskCompletedwhenCommandsis empty (e.g., update rejections / heartbeats). Tests are expanded to assert metadata presence across success and several error/edge cases.Written by Cursor Bugbot for commit 69c3140. This will update automatically on new commits. Configure here.